Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for GRUB patches from SUSE #321

Merged
merged 1 commit into from
Apr 8, 2024
Merged

Add support for GRUB patches from SUSE #321

merged 1 commit into from
Apr 8, 2024

Conversation

StollD
Copy link
Contributor

@StollD StollD commented Mar 10, 2024

Some GRUBs out there (Fedora, openSUSE) have an option that makes all paths relative to the default subvolume of the filesystem. This can be used to include /boot in your snapshots and roll them back without having to regenerate grub.cfg.

However, enabling that option will break grub-btrfs, because loading the kernel from a different snapshot requires the paths to be absolute. To make this work, GRUB has to be told explicitly to access the root subvolume when booting to a snapshot.

I hope this is acceptable, because it's essentially a workaround for a downstream patch of GRUB.

@StollD
Add support for GRUB patches from SUSE
ece8d87 
Some GRUBs out there (Fedora, openSUSE) have an option that makes all
paths relative to the default subvolume of the filesystem. This can be
used to include /boot in your snapshots and roll them back without
having to regenerate grub.cfg.

However, enabling that option will break grub-btrfs, because loading the
kernel from a different snapshot requires the paths to be absolute.

To make this work, GRUB has to be told explicitly to access the root
subvolume when booting to a snapshot.
@Schievel1
Copy link
Collaborator

hm, if that is a downsteam patch some distros apply to grub, couldn't this PR be applied as a downstream patch for those distros as well? I mean when the package is created.

@StollD
Copy link
Contributor Author

StollD commented Mar 11, 2024

I can only speak about Fedora, but grub-btrfs is not packaged there. I've been installing it with make install, so I figured I would propose this here. Especially since I don't know if it might apply to any other distro out there.

@Schievel1 Schievel1 requested a review from Antynea March 12, 2024 09:39
@Schievel1
Copy link
Collaborator

I have been considering this for a while now, but in the end I think grub-btrfs should stay distro agnostic.
Nevertheless this would work for me if you name the variable a bit different (not mentioning a distro but instead describe what it actually does) and maybe add a few lines about that feature in the readme.

@StollD
Copy link
Contributor Author

StollD commented Mar 27, 2024

I just noticed that I never actually linked the patchset I am talking about, so I'd like to apologize for that.

The SUSE_BTRFS_SNAPSHOT_BOOTING variable is not defined by me, it is defined by the patchset itself and enables the behaviour I described. I can't change its name, and I wouldn't want to document it in the readme because it is not defined by grub-btrfs.

I can add a comment or reword the commit to better explain the purpose of that variable though, whatever you prefer.

@Schievel1
Copy link
Collaborator

ooh so this is not something the grub-btrfs user has to set in the config, it is available to all scripts in /etc/grub.d?

@StollD
Copy link
Contributor Author

StollD commented Mar 27, 2024

Yeah, it is set in /etc/default/grub

@Schievel1 Schievel1 merged commit f682e17 into Antynea:master Apr 8, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Antynea Antynea Awaiting requested review from Antynea

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@StollD @Schievel1